Skip to content

feat: add new-sep scaffolding command#272

Merged
pcarleton merged 3 commits into
modelcontextprotocol:mainfrom
pugsatoshi:feat/new-sep-scaffold
May 13, 2026
Merged

feat: add new-sep scaffolding command#272
pcarleton merged 3 commits into
modelcontextprotocol:mainfrom
pugsatoshi:feat/new-sep-scaffold

Conversation

@pugsatoshi
Copy link
Copy Markdown
Contributor

@pugsatoshi pugsatoshi commented May 13, 2026

This pull request adds the new-sep <NNNN> CLI subcommand and a companion Claude Code skill that scaffolds a sep-NNNN.yaml requirement-traceability file for a new SEP — populating sep, spec_url (derived from the SEP's spec diff), and TODO requirements[] rows in the same style as src/scenarios/server/sep-2164.yaml.

Motivation and Context

Related #243.

SEP-2484 introduces the YAML format for requirement traceability. Currently, only sep-2164.yaml exists. Contributors must write the front matter from scratch, including the gh api ritual documented in AGENTS.md for extracting spec_url from docs/specification/draft/ diffs. This PR eliminates that manual work, allowing SEP-2484 to reach final status.

How Has This Been Tested?

Offline Scaffold from a Known Spec Path:

$ node dist/index.js new-sep 9999 --spec-path docs/specification/draft/server/resources.mdx
Wrote src/scenarios/server/sep-9999.yaml
Next steps:
  1. Edit the file to quote real normative sentences from the spec diff
     (and add a "#anchor" to spec_url if the requirement lives in a subsection)
  2. Implement the TypeScript scenario
  3. Register it in the appropriate suite list in src/scenarios/index.ts

$ cat src/scenarios/server/sep-9999.yaml
sep: 9999
spec_url: https://modelcontextprotocol.io/specification/draft/server/resources
requirements:
  - text: 'TODO: quote the normative sentence from the spec diff'
    check: sep-9999-todo
  - text: 'TODO: requirement that cannot be tested'
    excluded: 'TODO: reason'
    issue: https://github.com/modelcontextprotocol/conformance/issues/<NNNN>

Online lookup via GitHub API (modelcontextprotocol/modelcontextprotocol PR #2164):

$ node dist/index.js new-sep 9999 --pr 2164
Multiple spec files changed; picking docs/specification/draft/server/resources.mdx (most additions).
Resolved spec path: docs/specification/draft/server/resources.mdx
Wrote src/scenarios/server/sep-9999.yaml

The generated spec_url matches the page URL of the .mdx file that the PR changes. The output style is byte-identical to sep-2164.yaml.

--force guard verified (refuses to overwrite an existing yaml without the flag), --target override verified for client / server / authorization-server, and input validation (Invalid SEP number, Invalid --target, spec path must start with ...) verified.

Breaking Changes

None

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

pugsatoshi and others added 3 commits May 12, 2026 07:57
@pugsatoshi
feat: add new-sep scaffolding command
99f951b 
Related modelcontextprotocol#243.

This commit adds the new-sep <NNNN> subcommand, which writes the file src/scenarios/<target>/sep-<NNNN>.yaml. This file contains the "sep", "spec_url", and "requirements" rows in the same style as sep-2164.yaml, with single quotes and two spaces. The spec source resolves in order: --spec-url → --spec-path → GitHub API against modelcontextprotocol/modelcontextprotocol (explicit --pr or title search for "SEP-NNNN").
The target directory is inferred from the spec path (`server/`, `client/`, or `basic/authorization*`); the --target flag overrides this. It refuses to overwrite without the --force flag.
Token resolution reuses the "gh auth token" fallback pattern from tier-check.

It also ships the .claude/skills/new-sep directory, which drives the CLI. It fetches the spec diff via "gh api" and extracts RFC 2119 sentences. There is a warning against regex-only matching. Bullets inherit keywords from their lead-in sentence. It rewrites the placeholder rows per the severity rules in AGENTS.md. MUST/SHALL/REQUIRED are checked for FAILURE. SHOULD is checked for WARNING. MAY/OPTIONAL are excluded.

Signed-off-by: Satoshi Ito <satoshi.ito.tf@hitachi.com>
@pcarleton
dropped --pr arg, list changed files, support multi-file seps w url o…
db6052e 
…verride
@pcarleton
Consolidate SEP yamls into src/seps/ and tighten yaml ordering
d4aa04c 
- All sep-NNNN.yaml files now live in src/seps/ regardless of which
  scenario suite their checks run in
- Drop --target / inferTarget; output dir is fixed
- renderYaml: check rows emit `check:` first, excluded rows grouped at
  bottom with a blank-line separator
- SKILL.md: document key/row ordering, expand exclusion-confirmation step
- Move src/scenarios/server/sep-2164.yaml -> src/seps/
@pcarleton
Copy link
Copy Markdown
Member

pcarleton commented May 13, 2026

Thanks for this! I pushed a couple of tweaks on top after dry-running against SEP-2468. Here's a summary of what and why:

PR lookup simplified (db6052e)

  • The title search (SEP-NNNN in:title) returned two PRs for SEP-2468 (the SEP itself + a follow-up spec PR) and errored as "ambiguous." SEP numbers are PR numbers in the spec repo by convention, so dropped the search and --pr — the positional <number> is used directly. --spec-path covers the rare case where spec changes land elsewhere.
  • Added optional per-row url: (overrides file-level spec_url) for SEPs that touch multiple .mdx files. CLI now lists secondary spec files instead of silently picking one.

One yaml directory (d4aa04c)

  • All sep-NNNN.yaml go in src/seps/ instead of src/scenarios/{client,server,authorization-server}/. Traceability is SEP-scoped, not suite-scoped — a SEP with both client and server requirements shouldn't need two files. Dropped --target / inferTarget(). Per-row suite annotation is a follow-up once something actually consumes these.

YAML shape

  • check: is the first key on check rows so scanning the left margin shows every ID; excluded rows lead with text:.
  • All check: rows first, blank line, then excluded: rows grouped at the bottom.

Skill: exclusion handling — biggest behaviour change, fell out of the SEP-2468 run:

  • MAY/OPTIONAL produce no row. excluded: means "this MUST/SHOULD can't be tested"; a MAY isn't a test target, so listing it as excluded was a category error.
  • Exclusions now require user confirmation (new Step 6). On 2468, "client MUST record the issuer" is internal state — not wire-observable — but the old skill kept it as check:. An excluded: row is a permanent decision not to test a requirement, so the skill asks: one batch question for clearly-untestable rows, then per-row for borderline ones.
  • Borderline heuristics (internal-state verbs record/store/track; UI verbs display/show; precondition phrasing) are the patterns that tripped on 2468.
  • merge into <other-slug> handles preconditions — e.g. "MUST record the issuer" folds into "MUST compare iss against advertised" with text appended, rather than getting its own un-implementable check.
  • Step 8: skill now greps the target suite for an existing scenario to extend before suggesting a new file.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 13, 2026 •
edited
Loading

Open in StackBlitz

npx https://pkg.pr.new/@modelcontextprotocol/conformance@272

commit: d4aa04c

@pcarleton pcarleton merged commit 04dd6db into modelcontextprotocol:main May 13, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcarleton pcarleton pcarleton approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pugsatoshi @pcarleton